Level: an async option on load/reload/next/previous, and a microtask deferral by obiot · Pull Request #1647 · melonjs/melonJS

obiot · 2026-09-05T07:21:16Z

Closes #1646.

The timer

level.load() deferred its work with a timer so the current frame could unwind before the world is reset. The deferral is still needed — it is routinely called from a trigger handler mid-loop, and safeLoadLevel resets and destroys the very container the loop may be iterating, while state.stop() only sets a flag.

The timer itself is a 2011 artefact: that line and its comment trace to v0.9.0, four years before promises existed. Browsers clamp a timer to ≥1 s in a background tab, so a load queued as the tab hides sat behind that clamp. A microtask drains when the JS stack empties — the end of the rAF callback holding update and draw — so it unwinds the frame identically and is not clamped.

The no-loop branch stays synchronous exactly as before.

The `async` option

load, reload, next and previous each keep one name and gain a flag:

level.load("map1");                          // boolean, exactly as before
await level.load("map1", { async: true });   // Promise<boolean>
if (await level.next({ async: true })) { … }

options.onLoaded still fires either way, so the forms mix.

No type break. The signatures are preserved as JSDoc @overload pairs, not a boolean | Promise<boolean> union — a union fails every existing const ok: boolean = level.load(id) with TS2322, which I verified before choosing. The overload form compiles both that and the awaited call against the real emitted build:

const ok: boolean = level.load("map1");                       // still compiles
const a: boolean = await level.load("map1", { async: true });  // Promise<boolean>

Running out of levels still reports false rather than rejecting — reaching the end of a game is an ordinary outcome. An unknown level id throws synchronously in both forms: that is a typo, not a load failure, and it should not need await to surface.

The bounds check next and previous each spelled out is now a shared levelIdAt(offset) helper, so the two cannot drift.

Trigger stops rewriting its caller's options

The fade/mask path sequenced hide → load → reveal by replacing settings.onLoaded with its own function and calling the user's from inside — mutating an option object the caller owns. Awaiting the load removes the interception.

The viewport is deliberately re-read after the load: game.reset() reassigns app.viewport, which is precisely why the callback this replaces read it late.

The cost of putting the switch in the options

await level.load(id) without the flag is silent, because await true is valid JavaScript. It happens to be harmless today — the deferral is a single microtask queued before the await's continuation, so the load still runs first — but that is incidental ordering, not a contract. Documented on the LevelLoadOptions typedef and pinned by a test that records exactly this.

Tests

No spec called level.load() at all before this, so both files are new — 24 tests across the legacy contract, the flag, the scheduling, the reload/next/previous paths, and the trigger.

Nine mutations of the changed behaviour fail as they should: the flag ignored, the microtask reverted to a timer, the unknown id rejecting instead of throwing, the loop not stopped, the synchronous branch throwing instead of rejecting, next/previous off-by-one, the onLoaded wrap reintroduced, and a stale viewport captured before the load.

A tenth (rewriting the bounds helper as levelIdx[index] ?? null) survives, and is an equivalent mutant rather than a gap: out-of-range array access is already undefined, so the explicit bounds check is belt-and-braces. The observable behaviour is covered by the two off-by-one mutations.

The viewport guard is a source check rather than a behavioural test — the reveal only runs when the hide tween completes, which needs a live game loop the suite does not have. It was vacuous on the first attempt (the explanatory comment in the inspected slice contained the string it asserted on), so comment lines are stripped before matching, and it was re-verified against the mutation.

Also

reload() was documented as returning object — "the current level" — but returns whatever load() returns. The 2011 original returned nothing at all, so the declaration was never right. Corrected to boolean; getCurrentLevel() is the call that hands back the level object.

Verification

276 files, 6716 tests, 0 failures. Lint 0 errors, build clean, emitted overloads and legacy compatibility both checked against the built types.

`level.load()` deferred its work with a timer so the current frame could unwind before the world is reset. The deferral is still needed — it is routinely called from a trigger handler mid-loop, and `safeLoadLevel` resets and destroys the very container the loop may be iterating, while `state.stop()` only sets a flag — but the timer is a 2011 artefact. That line and its comment date to v0.9.0, four years before promises existed; there was never a macrotask semantic to preserve. Browsers clamp a timer to at least a second in a background tab, so a load queued as the tab hides was stranded behind that clamp. A microtask drains when the JS stack empties, which unwinds the frame just the same and is not clamped. The no-loop branch stays synchronous exactly as before: with no loop there is no frame to unwind, and deferring would change when the level exists for anyone loading one before the game starts. `loadAsync()` then returns that completion instead of discarding it. `load()` is unchanged and still returns `true` — the emitted type stays `boolean`, so a typed consumer doing `const ok: boolean = level.load(id)` keeps compiling, which is why this is a sibling rather than a changed return type. `options.onLoaded` still fires either way. An unknown level id throws synchronously rather than rejecting: that is a typo, not a load failure, and it should not need `await` to surface. `load()` rethrows a rejection on a clean stack so a failure still surfaces as an uncaught error the way it did under the timer, rather than as a silent unhandled rejection. `Trigger` stops rewriting its caller's options. Its fade/mask path sequenced hide → load → reveal by replacing `settings.onLoaded` with its own function and calling the user's from inside it; awaiting the load removes that interception. The viewport is deliberately re-read after the load — `game.reset()` reassigns `app.viewport`, which is exactly why the callback this replaces read it late. Tests: no spec called `level.load()` at all before this, so both files are new. Fifteen tests over the legacy contract, the new method, the scheduling, and the trigger paths; all eight mutations of the changed behaviour fail as they should, including a source guard on the viewport re-read, which the reveal path cannot cover behaviourally because its tween needs a live loop. Closes #1646 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🟡 Changes recommended

There are a few correctness/docs issues to address (notably loadAsync() error-surface consistency and a non-Markdown {@link ...} tag in the changelog).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR modernizes level-loading scheduling by replacing the legacy setTimeout deferral with a microtask-based deferral, and introduces an awaitable level.loadAsync() to let callers observe completion without changing the existing level.load() boolean-return contract. It also updates Trigger’s level-transition path to avoid mutating caller-owned settings.onLoaded, and adds new Vitest coverage for the behavior and scheduling.

Changes:

Add level.loadAsync(levelId, options): Promise<void> while keeping level.load()’s boolean return and “fire-and-forget” behavior.
Replace timer-based deferral with a microtask deferral when the game loop is running; preserve synchronous load behavior when no loop is running.
Refactor trigger level transitions to await the load (via loadAsync) instead of wrapping/overwriting settings.onLoaded, and add new tests.

File summaries

File	Description
packages/melonjs/src/level/level.js	Adds `loadAsync()`, refactors `load()` to delegate and rethrow failures, and replaces timer deferral with a microtask deferral when the loop is running.
packages/melonjs/src/renderable/trigger.js	Updates transition path to use `level.loadAsync(...).then(...)` for reveal instead of rewriting `settings.onLoaded`.
packages/melonjs/tests/level_load_async.spec.js	New tests for `loadAsync`, legacy `load()` contract, and microtask-vs-timer scheduling behavior.
packages/melonjs/tests/trigger_level_change.spec.js	New tests ensuring trigger transition behavior doesn’t overwrite caller callbacks and that load is deferred until hide completes.
packages/melonjs/CHANGELOG.md	Documents the new API and the background-tab timer clamp fix.

Review details

Files reviewed: 5/5 changed files
Comments generated: 4
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+		this.loadAsync(levelId, options).catch((error) => {
+			queueMicrotask(() => {
+				throw error;
+			});
+		});


+		safeLoadLevel(levelId, options);
+		return Promise.resolve();


+							.catch((error) => {
+								// same loudness as the fire-and-forget form
+								queueMicrotask(() => {
+									throw error;
+								});
+							});


 ## [20.4.0] (melonJS 2) - _unreleased_

 ### Added
+- `level.loadAsync(levelId, options)` — the same load as {@link level.load}, resolving once the level is in the world instead of discarding the completion. `options.onLoaded` still fires, so the two forms mix freely, and `load()` is unchanged and still returns `true`. An unknown level id throws synchronously rather than rejecting: that is a typo, not a load failure, and it should not need `await` to surface ([#1646](https://github.com/melonjs/melonJS/issues/1646))


`loadAsync()` on its own left the other three loading calls with no awaitable form, so a game could await its first level but not a reload or a level transition. Each twin resolves with exactly what its synchronous counterpart returns, which makes a port mechanical: `if (level.next())` becomes `if (await level.nextAsync())`. That is why `loadAsync()` now resolves `true` rather than `void` — the rule is worth more than the slightly noisier type. Running out of levels resolves `false` without loading anything rather than rejecting: `next()` returns `false` there, and reaching the end of a game is an ordinary outcome, not an error. The four originals are untouched, and their emitted types are unchanged — `load`, `next` and `previous` still declare `boolean`. Not included: `reload()` declares `object` from a stale `@returns {object} the current level`, but it returns whatever `load()` returns. Correcting that would change an emitted type, which is the one thing this change set is careful not to do, so it is left alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

obiot · 2026-09-05T08:32:25Z

Added the three missing twins — reloadAsync(), nextAsync(), previousAsync(). loadAsync() alone left a game able to await its first level but not a reload or a level transition.

Each resolves with exactly what its synchronous counterpart returns, so a port is mechanical:

if (level.next())              →   if (await level.nextAsync())

That is why loadAsync() now resolves true rather than void — the rule is worth more than the marginally noisier type.

Running out of levels resolves false without loading anything rather than rejecting. next() returns false there, and reaching the end of a game is an ordinary outcome, not an error.

Emitted types, verified from build/level/level.d.ts — all four originals unchanged:


`load`	`boolean`	`loadAsync` → `Promise<boolean>`
`reload`	`object`	`reloadAsync` → `Promise<boolean>`
`next`	`boolean`	`nextAsync` → `Promise<boolean>`
`previous`	`boolean`	`previousAsync` → `Promise<boolean>`

Six more tests, and five mutations of the new behaviour all fail as they should: nextAsync resolving true at the end, loading past the end, a previousAsync off-by-one at the start, reloadAsync not reloading, and loadAsync resolving undefined.

Noticed but deliberately not fixed: reload() declares object above, from a stale @returns {object} the current level — it actually returns whatever load() returns. Correcting it would change an emitted type, which is precisely what this PR is careful not to do, so it is left for a separate call.

Suite now 276 files, 6713 tests, 0 failures.

Copilot

🟡 Changes recommended

It introduces unconditional queueMicrotask usage (risking runtime ReferenceError in unsupported environments) and the PR description’s stated scope conflicts with the included async twin APIs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

packages/melonjs/src/level/level.js:198

load() rethrows rejections using queueMicrotask, but this global is not guaranteed to exist in all runtimes. If it’s missing, a rejected loadAsync() will cause a ReferenceError here instead of surfacing the original failure. Consider a small fallback to setTimeout when queueMicrotask is unavailable.

	load(levelId, options) {
		// Fire-and-forget by contract: this returns `true`, not the promise, so
		// existing (including typed) callers are unaffected. Use `loadAsync()`
		// to await the load. The rejection is rethrown on a clean stack so a
		// failure still surfaces as an uncaught error the way it did when the
		// deferral was a timer, rather than as a silent unhandled rejection.
		this.loadAsync(levelId, options).catch((error) => {
			queueMicrotask(() => {
				throw error;
			});
		});

packages/melonjs/src/renderable/trigger.js:209

This catch path rethrows via queueMicrotask, which may be undefined in some runtimes; in that case the code would throw a ReferenceError and potentially mask the real level-load failure. Using a simple fallback (e.g. setTimeout) keeps the intended “uncaught” loudness without requiring queueMicrotask support.

							})
							.catch((error) => {
								// same loudness as the fire-and-forget form
								queueMicrotask(() => {
									throw error;
								});
							});

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Lite

 ### Added
+- `level.loadAsync()`, `reloadAsync()`, `nextAsync()` and `previousAsync()` — awaitable twins of the four level-loading calls, resolving once the level is actually in the world instead of discarding the completion. Each resolves with exactly what its synchronous twin returns, so a port is mechanical: `if (level.next())` becomes `if (await level.nextAsync())`, and running out of levels still resolves `false` rather than rejecting. `options.onLoaded` still fires, so the two forms mix freely, and the originals are unchanged — `load()` still returns `true`, and its emitted type is still `boolean`. An unknown level id throws synchronously rather than rejecting: that is a typo, not a load failure, and it should not need `await` to surface ([#1646](https://github.com/melonjs/melonJS/issues/1646))


`@returns {object} the current level` was never true. `reload()` is `return this.load(...)`, and `load()` returns `true` — and the 2011 original returned nothing at all, so the declaration has been wrong for the method's entire life. `getCurrentLevel()` is the call that hands back the level object. This corrects the emitted type from `object` to `boolean`. A `const lvl: object = level.reload()` that compiled while receiving `true` now fails to compile, which surfaces a bug that was already there rather than introducing one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🔵 Needs a closer look

It changes core level-loading scheduling and trigger transition sequencing in a way that can have subtle runtime/event-loop effects best validated by a human reviewer.

Review details

Files reviewed: 5/5 changed files
Comments generated: 0 new
Review effort level: Lite

Review catch. The deferred branch turned a failing load into a rejection, but the synchronous one let the exception escape the call — so the error surface depended on whether the loop happened to be running, and `loadAsync(...).catch()` could never see the synchronous case, because the throw beat the handler being attached. The unknown-id check still throws synchronously, before either branch: that is a typo rather than a load failure, and should not need `await`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🟡 Changes recommended

level.load() now forces some previously synchronous failures (when the loop isn’t running) to become asynchronous throws via loadAsync().catch(...), which is a behavior/contract change that should be addressed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 5/5 changed files
Comments generated: 1
Review effort level: Lite

 	load(levelId, options) {
+		// Fire-and-forget by contract: this returns `true`, not the promise, so
+		// existing (including typed) callers are unaffected. Use `loadAsync()`
+		// to await the load. The rejection is rethrown on a clean stack so a
+		// failure still surfaces as an uncaught error the way it did when the
+		// deferral was a timer, rather than as a silent unhandled rejection.
+		this.loadAsync(levelId, options).catch((error) => {
+			queueMicrotask(() => {
+				throw error;
+			});
+		});
+		return true;


Eight loading methods for four operations was too much surface. The switch moves into the options object the calls already take, so `load`, `reload`, `next` and `previous` each keep one name and gain a flag. No type break, which is the part that had to be got right. The signatures are preserved as JSDoc `@overload` pairs rather than a `boolean | Promise<boolean>` union — a union would fail every existing `const ok: boolean = level.load(id)`, verified with tsc. The overload form compiles both that and `await level.load(id, { async: true })` against the real emitted build. The bounds check `next` and `previous` each spelled out is now a shared `levelIdAt(offset)` helper, so the two cannot drift. The cost of putting the switch in the options is that `await level.load(id)` without the flag is silent — `await true` is valid. It happens to be harmless today, since the deferral is a single microtask queued before the await's continuation, so the load still runs first; that is incidental ordering rather than a contract. Documented on the options typedef and pinned by a test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

obiot · 2026-09-05T08:59:13Z

Responses to the review, plus a design change since it was written.

queueMicrotask → Promise.resolve().then(() => { throw error }) (two comments): declining. The two are not equivalent for this purpose, measured in the browser:

queueMicrotask(() => { throw e })          → ["uncaught-error"]
Promise.resolve().then(() => { throw e })  → ["unhandled-rejection"]

That rethrow exists specifically to preserve the uncaught error the timer-based version produced. The suggested form would turn it into an unhandled rejection, which is the behaviour change the code is there to avoid. On availability: queueMicrotask is baseline in every browser since 2018 and Node 11, comfortably older than the ES2022 target this package already requires.

loadAsync() error-surface inconsistency: valid, fixed. The deferred branch produced a rejection while the synchronous one let the exception escape the call, so the surface depended on state.isRunning() — and .catch() could never see the synchronous case, since the throw beat the handler being attached. The synchronous branch now returns a rejected promise, with a test asserting both branches reject identically. Good catch.

{@link ...} in the changelog: already gone — the entry had been rewritten before the review ran.

PR description out of step with scope: it was, and it is now further out of step, because the API changed after this review. The description has been rewritten.

Design change: the four *Async siblings are gone. Eight loading methods for four operations was too much surface, so the switch moved into the options object: level.load(id, { async: true }), and the same on reload/next/previous.

The signatures are preserved as JSDoc @overload pairs rather than a boolean | Promise<boolean> union — the union form fails every existing const ok: boolean = level.load(id) with TS2322, verified with tsc before choosing. The overload form compiles both the legacy call and the awaited one against the real emitted build.

Copilot

🟡 Changes recommended

There are a few correctness/robustness issues in the newly added docs/tests and a missing queueMicrotask fallback that could cause runtime failures in unsupported environments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

packages/melonjs/src/renderable/trigger.js:208

queueMicrotask is used here to rethrow load failures, but it isn’t feature-detected or polyfilled in this repo. In runtimes without queueMicrotask, this catch handler will throw a ReferenceError and may mask the original error. Consider falling back to setTimeout for the rethrow path.

								// same loudness as the fire-and-forget form
								queueMicrotask(() => {
									throw error;
								});

Files reviewed: 5/5 changed files
Comments generated: 3
Review effort level: Lite

+				queueMicrotask(() => {
+					throw error;
+				});


+ * Note that awaiting a call WITHOUT `async: true` is not an error — `await true`
+ * is valid and resolves immediately — so the level will not be loaded yet. Pass
+ * the flag whenever you intend to await.


+		const load = triggerSource.indexOf(
+			"load(gotolevel, { ...settings, async: true })",
+		);
+		const reveal = triggerSource.indexOf("addCameraEffect", load);


Left over from the rename to the `async` option. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

`melonjs-tilemaps` described the deferral as a `setTimeout` and told the reader the only way to sequence work after a load was `onLoaded` or `LEVEL_LOADED`. Both are now out of date: the deferral is a microtask, and `async: true` gives a promise to await. Also states the caveat that comes with putting the switch in the options — `await level.load(id)` without the flag returns a boolean, so it does not await the load. It happens to finish first today, because the deferral is a single microtask queued ahead of the await's continuation, but that is incidental ordering rather than a contract, and the skills say so rather than implying either that it is safe or that it is broken. `melonjs-3d-assets` gains the `async` row in its `level.load` options table and the same note; glTF/GLB scenes load through the same call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🟢 Approval recommended

The changes are cohesive, preserve legacy behavior, and are backed by targeted tests covering the new async contract, scheduling, and Trigger sequencing.

Review details

Files reviewed: 7/7 changed files
Comments generated: 0 new
Review effort level: Lite

Copilot

🔵 Needs a closer look

The new queueMicrotask-based rethrow paths should defensively handle runtimes where queueMicrotask is unavailable to avoid masking failures with a ReferenceError/unhandled rejection.

Review details

Suppressed comments (2)

packages/melonjs/src/level/level.js:270

The fire-and-forget path rethrows failures via queueMicrotask, but the project explicitly avoids bundling polyfills. If queueMicrotask is missing at runtime, this handler can throw a ReferenceError and potentially hide the original error (or produce an unhandled rejection). Consider falling back to setTimeout when queueMicrotask is unavailable.

				queueMicrotask(() => {
					throw error;

packages/melonjs/src/renderable/trigger.js:207

queueMicrotask is used to rethrow load failures, but it is a relatively new platform API and the engine intentionally avoids shipping polyfills. If queueMicrotask is unavailable in a runtime that can still parse the bundle, this catch handler will throw a ReferenceError and can turn the original failure into an unhandled rejection (or mask it). Consider a small fallback to setTimeout when queueMicrotask is missing.

								queueMicrotask(() => {
									throw error;

Files reviewed: 8/8 changed files
Comments generated: 0 new
Review effort level: Lite

The `LevelLoadOptions` note claimed that awaiting without `async: true` leaves the level unloaded. It does not: with no loop the load is already synchronous, and with one running the deferral is a single microtask queued ahead of the await's continuation, so it finishes first either way. The point stands — there is no completion point to await without the flag — but the ordering is incidental and the doc now says so instead of promising the opposite. The same wording was already fixed in the skills; this is the copy that was missed. The trigger guard searched for the full call text including argument spacing, so reformatting or an added option would have failed it for no reason. Anchored on `load(gotolevel` instead — still fails when the viewport is captured before the load, which is the only thing it is for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

obiot · 2026-09-05T10:41:27Z

Second round.

Options doc claiming the level is unloaded when you await without the flag — correct, fixed. That is the same error I had already corrected in the skills and missed in this copy: with no loop the load is synchronous, and with one running the deferral is a single microtask queued ahead of the await's continuation, so it finishes first either way. The point still stands — without the flag there is no completion point to await — but the ordering is incidental, and the doc now says that rather than the opposite.

Brittle source-substring test — fair, fixed. Anchored on load(gotolevel instead of the full argument text, so reformatting or an added option cannot fail it. Re-verified that it still fails when the viewport is captured before the load, which is the only thing it exists for.

load() delegating to loadAsync() losing the synchronous throw — resolved, though by redesign rather than by patch. loadAsync() no longer exists: the switch moved into the options object, and the no-loop non-async path is now literally

safeLoadLevel(levelId, options);
return true;

so a setup-time failure still throws synchronously and try { level.load(id) } catch still works. The promise form wraps the same call and rejects instead, which keeps the two consistent — that was the earlier review comment, and both are now satisfied at once.

queueMicrotask fallback — still declining, same reasoning as before. It is baseline in Chrome 71, Firefox 69, Safari 12.1 and Node 11, all from 2018–2019; ES2022 syntax this package already emits (class fields, private members) needs strictly newer engines than that, so there is no runtime that can parse the bundle but lacks the function. Adding a setTimeout fallback would also reintroduce the background-tab clamp on the error path this PR exists to remove. Worth restating that the choice is not stylistic: measured in-browser, queueMicrotask(() => { throw e }) produces an uncaught error while Promise.resolve().then(() => { throw e }) produces an unhandled rejection, and preserving the former is the point.

Copilot

🔵 Needs a closer look

New queueMicrotask usage is unguarded and not polyfilled in this repo, which can cause runtime TypeErrors that mask the original load error in environments lacking queueMicrotask.

Review details

Suppressed comments (2)

packages/melonjs/src/level/level.js:276

queueMicrotask is used to rethrow async load failures, but it’s not polyfilled anywhere in this repo. In runtimes where queueMicrotask is undefined, this will throw a TypeError and can mask the original load failure. Consider a small fallback so errors remain loud without introducing a new hard runtime requirement.

			deferred.catch((error) => {
				queueMicrotask(() => {
					throw error;
				});
			});

packages/melonjs/src/renderable/trigger.js:209

This .catch rethrows via queueMicrotask, but queueMicrotask is not polyfilled in melonJS (polyfills are intentionally Canvas/DOM-only). If a consumer runs in an environment without queueMicrotask, this handler itself throws and may hide the original level-load error. Add a small fallback (e.g., to setTimeout) to keep the error reporting robust.

							.catch((error) => {
								// same loudness as the fire-and-forget form
								queueMicrotask(() => {
									throw error;
								});
							});

Files reviewed: 8/8 changed files
Comments generated: 0 new
Review effort level: Lite

**TMX.** Every test stubbed a glTF scene, so `safeLoadLevel`'s format branch was only ever exercised on the non-TMX arm — and Tiled maps are the main use of `level.load`. A real map now loads in both forms, passed inline through the loader's `data` field so it needs no fixture file. The map carries an object group, because `flatten: false` wrapping it in a named `Container` is behaviour only `loadTMXLevel` produces: routing a map through the generic `addTo` arm passes the whole options object as its positional `flatten` argument and flattens everything, which that assertion now catches. **The trigger reveal.** Previously source-guarded, because its tween needed a live loop. Driving the tween by hand with `_onTick` removes that, so the sequencing is asserted for real: the load happens, then the reveal, and on the viewport that exists AFTER the load — `game.reset()` reassigns it. The test runs with the loop RUNNING; with it stopped the load is synchronous and the ordering proves nothing. **LEVEL_LOADED and onLoaded ordering.** Both must land before the promise resolves, which is what a caller awaiting the load then reading world state depends on. **A throwing callback.** The async form rejects, and the boolean form with no loop still throws synchronously. Three mutations that survived the first pass were equivalent mutants rather than gaps — a microtask queued after the load's own microtask still runs after it, so "emit late" and "reveal without waiting" needed genuinely-late variants (`setTimeout`, and a synchronous reveal) to express the bug. Both fail now, as does disabling the TMX arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🟢 Approval recommended

The changes are internally consistent with the stated compatibility goals, and the new/updated behavior is thoroughly covered by targeted tests for scheduling, ordering, and trigger integration.

Review details

Files reviewed: 8/8 changed files
Comments generated: 0 new
Review effort level: Lite

`queueMicrotask` is gone. It was also the wrong thing to reach for: it is not an ECMAScript feature at all — it comes from the HTML spec, declared in `lib.dom.d.ts`, not in any `lib.es2022`. The argument that the ES2022 target implied its presence was simply wrong, however true the conclusion happened to be for real runtimes. The deferral goes back to a timer, through `utils.function.defer`, which is what `state`, `Container` and `timer` already schedule with. That also restores the guarantee the 2011 timer actually provided: a macrotask cannot run inside another, so the load lands after the current frame whatever the frame does. The microtask version only landed there while the whole update-and-draw path stayed synchronous — true today, but it would have started running mid-frame, silently, the day anything in that path awaited. The fire-and-forget path gets simpler rather than more complex: no promise is created, so nothing can swallow a failure, and a throw inside the timer lands on an empty stack as the uncaught error it has always been. The rethrow machinery is deleted outright. The trade is the background-tab clamp, which comes back — that CHANGELOG entry is removed rather than left claiming a fix that no longer applies. Consequence for the docs, in three places: awaiting without `async: true` now genuinely does not wait. The microtask version happened to complete first by queue ordering; a timer does not. The typedef, both skills and the test all say so plainly now, which is the version I described before measuring and then had to walk back — it is true again, for a different reason. Adds unit tests for `defer` itself, which had none despite being public API and load-bearing in five modules: that it does not run synchronously, that it lands on a macrotask rather than a microtask, that it binds `thisArg` and forwards arguments, that its handle cancels, and that a throw inside it surfaces as an uncaught error rather than a rejection — the property the fire-and-forget load now depends on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🟡 Changes recommended

There are verified doc/test inconsistencies around “microtask” vs timer/macrotask deferral, including a trigger test that doesn’t yield to a macrotask even though the load is setTimeout-deferred.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (3)

packages/melonjs/src/level/level.js:335

This JSDoc refers to a microtask deferral, but reload() delegates to level.load(), which defers via setTimeout(…, 0) when the loop is running. Update the wording to avoid misleading API docs.

	 * While the game loop is running the load is deferred to a microtask, so this

packages/melonjs/src/level/level.js:374

This JSDoc refers to a microtask deferral, but next() ultimately uses level.load(), which defers via setTimeout(…, 0) when the loop is running. Update the wording to match the actual scheduling semantics.

	 * While the game loop is running the load is deferred to a microtask, so this

packages/melonjs/src/level/level.js:415

This JSDoc refers to a microtask deferral, but previous() ultimately uses level.load(), which defers via setTimeout(…, 0) when the loop is running. Update the wording to match the actual scheduling semantics.

	 * While the game loop is running the load is deferred to a microtask, so this

Files reviewed: 9/9 changed files
Comments generated: 3
Review effort level: Lite

+		// let the load's microtask and the reveal chained after it settle
+		for (let i = 0; i < 4; i++) {
+			await Promise.resolve();
+		}


+	 * (will also create all level defined entities, etc..)
 	 *
-	 * // ...
+	 * While the game loop is running the load is DEFERRED to a microtask, so


+ * The deferral in `level.load()` dates to 2011 and used a timer because that
+ * was the only way to defer at the time. It is still needed — `level.load()` is
+ * routinely called from inside the loop, and `safeLoadLevel` resets and
+ * destroys the very container the loop may be iterating — but it is now a
+ * microtask, and `async: true` hands that completion back instead of a boolean.


The promise branch wrapped the deferral in a `new Promise` executor with a try/catch to funnel a throw into `reject`. A `.then` already turns a throw into a rejection, so a small `nextTask()` helper — a promise that settles on the next task, through the engine's own `defer` — removes the executor, the try/catch and the manual resolve/reject. Deliberately not converted further. Nothing in the level lifecycle is actually asynchronous: both `addTo` implementations, glTF and TMX, are fully synchronous. Making `safeLoadLevel` async would buy syntax and cost atomicity — it currently runs to completion inside one task, so the world is never observable half-built, and spreading it across microtask ticks would give up that property for nothing. The fire-and-forget branch keeps calling `defer` directly. Routing it through `nextTask()` would create a promise, and a promise is exactly what must not exist there: with nothing holding it, a failed load would become a silent unhandled rejection instead of an uncaught error. Adds the test that made the refactor honest. The existing macrotask test drives the fire-and-forget path, which calls `defer` directly, so it never touched the promise branch — `nextTask()` could have reverted to a microtask unnoticed. Now pinned for both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🔵 Needs a closer look

The implementation and inline API docs describe timer/macrotask deferral, which conflicts with the PR description’s “microtask deferral” claim and includes multiple “microtask” doc strings that don’t match the actual scheduling.

Review details

Suppressed comments (5)

packages/melonjs/src/level/level.js:215

The docs here say the load is deferred to a microtask while the loop is running, but the implementation below schedules via defer, which is a setTimeout(..., 0) macrotask. This should say timer/macrotask to match real behavior.

	 * While the game loop is running the load is DEFERRED to a microtask, so
	 * this returns before anything is in the world. Sequence follow-up work from
	 * `options.onLoaded`, from an `event.LEVEL_LOADED` listener, or by passing
	 * `async: true` and awaiting the promise that overload returns.

packages/melonjs/src/level/level.js:345

This comment says reload is deferred to a microtask, but level.load() defers via defer (setTimeout), i.e. a timer/macrotask. Updating this avoids misleading API docs.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:384

This comment says next() is deferred to a microtask, but the deferral is a timer/macrotask via defer (setTimeout). Please align wording with the actual scheduling semantics.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:270

The PR description/linked issue text calls out replacing the old setTimeout deferral with a microtask to avoid background-tab timer clamping, but this implementation explicitly keeps using a timer/macrotask via defer (which is setTimeout(..., 0)). That means background clamping behavior remains; either the PR description/title needs updating, or the scheduling should be changed to an actual microtask (e.g. queueMicrotask/Promise.resolve().then(...)) and the associated tests updated accordingly.

		// A TIMER, through the engine's own `defer`, and deliberately not a
		// microtask. A macrotask cannot run inside another, so the load lands
		// after the current frame whatever the frame does. A microtask only
		// lands there while the whole update-and-draw path stays synchronous —
		// true today, but it would start running mid-frame the day anything in
		// that path awaits, silently and with nothing to catch it.

packages/melonjs/src/level/level.js:425

This comment says previous() is deferred to a microtask, but the deferral is a timer/macrotask via defer (setTimeout). Please align wording with the actual scheduling semantics.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Lite

One definition, one call site — the same shape as the `scheduleLoad` helper removed earlier. A private function plus a JSDoc block to name a three-line idiom is more ceremony than the name is worth. The `.then` structure stays, and the comment now says why it is load-bearing rather than incidental: a throw from inside `defer`'s callback escapes the promise executor entirely, leaving the promise pending forever. Raised one level up, in `.then`, it rejects — which is what removes the try/catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🔵 Needs a closer look

There are documented semantics claiming “microtask” deferral (and PR text referencing a microtask switch) that conflict with the actual timer/macrotask scheduling used in the implementation and tests.

Review details

Suppressed comments (6)

packages/melonjs/src/level/level.js:202

This JSDoc says the deferred branch is a “microtask”, but the implementation explicitly schedules via utils.function.defer (setTimeout), i.e. a timer/macrotask. The docs should match the actual scheduling semantics.

	 * While the game loop is running the load is DEFERRED to a microtask, so
	 * this returns before anything is in the world. Sequence follow-up work from
	 * `options.onLoaded`, from an `event.LEVEL_LOADED` listener, or by passing
	 * `async: true` and awaiting the promise that overload returns.

packages/melonjs/src/level/level.js:336

This overload doc claims the load is deferred to a microtask, but level.load() actually defers via utils.function.defer (setTimeout) when the loop is running. Update wording to avoid misleading callers about scheduling and background-tab timer clamping.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:375

This doc mentions “microtask” deferral, but next()/previous() just delegate to level.load(), which defers via utils.function.defer (setTimeout) while running. Align the wording with the actual macrotask/timer behavior.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:416

This doc mentions “microtask” deferral, but previous() delegates to level.load(), which defers via utils.function.defer (setTimeout) while running. Update wording to reflect the macrotask/timer scheduling.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/tests/level_load_async.spec.js:21

The file header comment says level.load() deferral is “now a microtask”, but the code under test (and several assertions below) treats it as timer/macrotask deferral via utils.function.defer (setTimeout). This comment should be corrected to avoid contradicting the tests’ intent.

 * The deferral in `level.load()` dates to 2011 and used a timer because that
 * was the only way to defer at the time. It is still needed — `level.load()` is
 * routinely called from inside the loop, and `safeLoadLevel` resets and
 * destroys the very container the loop may be iterating — but it is now a
 * microtask, and `async: true` hands that completion back instead of a boolean.

packages/melonjs/src/level/level.js:256

The PR description/title claims level loading switched from a timer to a microtask to avoid background-tab timer clamping, but the implementation here explicitly keeps a timer/macrotask (via utils.function.defer/setTimeout) and even calls out “deliberately not a microtask”. Either update the PR description to match the shipped behavior, or change the scheduling to a true microtask and adjust tests/docs accordingly.

		// A TIMER, through the engine's own `defer`, and deliberately not a
		// microtask. A macrotask cannot run inside another, so the load lands
		// after the current frame whatever the frame does. A microtask only
		// lands there while the whole update-and-draw path stays synchronous —
		// true today, but it would start running mid-frame the day anything in

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Lite

`triggerEvent` schedules the load through `defer`, so a test that fires a trigger leaves a timer pending. `afterAll` then destroyed the application, and the callback landed on a torn-down world: TypeError: Cannot read properties of undefined (reading 'set') World.reset physics/world.js:266 safeLoadLevel level/level.js:23 Vitest reports that as a fatal unhandled error while still counting every test as passed, so the suite reads green and the job fails. The level spec already flushed for this reason when the deferral moved back to a timer; the trigger spec was missed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🔵 Needs a closer look

Several updated JSDoc comments and the PR narrative describe microtask-based deferral, but the implementation still defers via defer/setTimeout (timer/macrotask), creating user-facing documentation and expectation mismatches that should be resolved.

Review details

Suppressed comments (5)

packages/melonjs/src/level/level.js:202

The JSDoc says the load is deferred to a microtask while the loop is running, but the implementation explicitly defers via utils.function.defer (setTimeout), i.e. a timer/macrotask. This mismatch is likely to mislead users reading the API docs about when the world is actually populated.

	 * While the game loop is running the load is DEFERRED to a microtask, so
	 * this returns before anything is in the world. Sequence follow-up work from
	 * `options.onLoaded`, from an `event.LEVEL_LOADED` listener, or by passing
	 * `async: true` and awaiting the promise that overload returns.

packages/melonjs/src/level/level.js:336

This docstring says reload is deferred to a microtask when the loop is running, but level.load/reload defer via utils.function.defer (setTimeout), i.e. a timer/macrotask. Keeping the docs consistent with the actual scheduling avoids confusion around ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:375

This docstring says next() is deferred to a microtask when the loop is running, but the actual scheduling is via utils.function.defer (setTimeout), i.e. a timer/macrotask. The wording should match the real behavior so callers know what they can safely sequence after the call.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:416

This docstring says previous() is deferred to a microtask when the loop is running, but the actual scheduling is via utils.function.defer (setTimeout), i.e. a timer/macrotask. Aligning the docs with behavior matters for callers relying on precise ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:257

The PR description/issue text says the setTimeout deferral was replaced with a microtask to avoid background-tab timer clamping, but the implementation still defers through utils.function.defer, which is setTimeout(..., 0). If the goal is to eliminate timer clamping, this scheduling strategy won’t achieve that; if the goal changed to “centralize on defer”, the PR title/description (and issue closure) should be updated to match.

		// A TIMER, through the engine's own `defer`, and deliberately not a
		// microtask. A macrotask cannot run inside another, so the load lands
		// after the current frame whatever the frame does. A microtask only
		// lands there while the whole update-and-draw path stays synchronous —
		// true today, but it would start running mid-frame the day anything in
		// that path awaits, silently and with nothing to catch it.

Files reviewed: 9/9 changed files
Comments generated: 0 new
Review effort level: Lite

`container`, `setViewportBounds` and `flatten` are pre-existing defaults this change does not touch, but nothing called `level.load` before, so nothing pinned them either — and they are the contract a game gets when it passes no options at all. - `container` defaults to the application's world - `setViewportBounds` defaults to TRUE on the TMX arm, which is what calls `viewport.setBounds`; asserted against an explicit `false` so the default is what is being measured rather than the code path merely running - `flatten` defaults to the application's `mergeGroup` rather than to a constant, so it is pinned in both positions Flipping each default fails a test. Replacing the container default with a different container fails too, though loudly — the assertion is a direct identity check against the app's world. The TMX map fixture moves to the top-level hook. It was registered inside a sibling describe, so the new tests only saw it through declaration order, which is not something to rely on. That adds a third level to the registry, so the next/previous boundary tests now name the last level instead of assuming which one it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N

Copilot

🟡 Changes recommended

There are correctness/documentation inconsistencies around timer-vs-microtask deferral and at least one Trigger transition sequencing edge case (reveal ordering/behavior) plus a test that awaits only microtasks despite macrotask scheduling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (7)

Previously missed (1) — in code that hasn't changed since the last review.

packages/melonjs/src/level/level.js:180

The summary line for level.load says it "return[s] a promise" unconditionally, but the legacy (non-async) overload still returns a boolean. This wording is likely to confuse readers skimming docs/API reference.

packages/melonjs/src/level/level.js:202

Docs say the deferred load is "a microtask", but the implementation explicitly schedules via defer()/setTimeout(..., 0) (a macrotask). Please align the JSDoc wording with the actual scheduling semantics.

	 * While the game loop is running the load is DEFERRED to a microtask, so
	 * this returns before anything is in the world. Sequence follow-up work from
	 * `options.onLoaded`, from an `event.LEVEL_LOADED` listener, or by passing
	 * `async: true` and awaiting the promise that overload returns.

packages/melonjs/tests/trigger_level_change.spec.js:164

This test only awaits microtasks, but level.load(..., { async: true }) defers the actual load via utils.function.defer, which uses setTimeout(..., 0) (a macrotask). Without awaiting a timer tick, loaded/seen may still be unchanged when assertions run.

		// let the load's microtask and the reveal chained after it settle
		for (let i = 0; i < 4; i++) {
			await Promise.resolve();
		}

packages/melonjs/src/level/level.js:336

This JSDoc mentions deferring to "a microtask", but level.load currently defers via defer()/setTimeout when the loop is running. Keeping these docs consistent avoids incorrect assumptions about ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:375

This JSDoc mentions deferring to "a microtask", but level.load currently defers via defer()/setTimeout when the loop is running. Keeping these docs consistent avoids incorrect assumptions about ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:416

This JSDoc mentions deferring to "a microtask", but level.load currently defers via defer()/setTimeout when the loop is running. Keeping these docs consistent avoids incorrect assumptions about ordering.

	 * While the game loop is running the load is deferred to a microtask, so this
	 * returns before anything is in the world — see {@link level.load}.

packages/melonjs/src/level/level.js:257

The PR description/title emphasize switching the deferral to a microtask, but the implementation here explicitly chooses a timer/macrotask via defer() (setTimeout) and calls out "deliberately not a microtask". If the intended behavior is macrotask deferral, the PR description/title should be updated; if microtask deferral is still desired, the implementation/tests/docs should be revisited for consistency.

		// A TIMER, through the engine's own `defer`, and deliberately not a
		// microtask. A macrotask cannot run inside another, so the load lands
		// after the current frame whatever the frame does. A microtask only
		// lands there while the whole update-and-draw path stays synchronous —
		// true today, but it would start running mid-frame the day anything in
		// that path awaits, silently and with nothing to catch it.

Files reviewed: 9/9 changed files
Comments generated: 1
Review effort level: Lite

+						level
+							.load(gotolevel, { ...settings, async: true })
+							.then(() => {
+								// re-read AFTER the load: `game.reset()` reassigns
+								// `app.viewport`, so a viewport captured before it


Copilot AI lite review requested due to automatic review settings September 5, 2026 07:21

Copilot started reviewing on behalf of obiot September 5, 2026 07:21 View session